Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorporate CI and ARM feedback from azure-rest-api-specs to v2024-08-12-preview API #3727

Open
wants to merge 27 commits into
base: master
Choose a base branch
from

Conversation

cadenmarchese
Copy link
Collaborator

@cadenmarchese cadenmarchese commented Jul 26, 2024

Which issue this PR addresses:

https://issues.redhat.com/browse/ARO-4382

What this PR does / why we need it:

I opened up a PR to azure-rest-api-specs with our new API version: Azure/azure-rest-api-specs#30004

These are the changes that were required to unblock CI / resolve breaking change and other CI failures.

Test plan for issue:

Is there any documentation that needs to be updated for this PR?

How do you know this will function as expected in production?

@cadenmarchese cadenmarchese added the chainsaw Pull requests or issues owned by Team Chainsaw label Jul 26, 2024
@cadenmarchese cadenmarchese force-pushed the cadenmarchese/ARO-4382/api-changes-1 branch from 83f43ef to 7a69b14 Compare July 30, 2024 20:19
tsatam
tsatam previously approved these changes Jul 31, 2024
@github-actions github-actions bot added needs-rebase branch needs a rebase and removed ready-for-review labels Jul 31, 2024
Copy link

Please rebase pull request.

out.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities[i].ResourceID = oc.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities[i].ResourceID
out.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities[i].ClientID = oc.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities[i].ClientID
out.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities[i].ObjectID = oc.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities[i].ObjectID
for k := range oc.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic should be able to be simplified similar to how the tags are formed, isn't it?

for k, v := range oc.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities {
    out.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities[k].ClientId == v.ClientId
...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried that first, but was unable to get it to compile:

cannot assign to struct field out.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities[k].ResourceID in map

etc...

cadenmarchese and others added 26 commits September 27, 2024 13:20
this was wrong in the readme
top level fields must not have additionalproperties
it was requested that we use the latest version of
common types. This involves some changes to our examples
to match the UUID expected.
avoids python client generation issues
@cadenmarchese cadenmarchese force-pushed the cadenmarchese/ARO-4382/api-changes-1 branch from 6675b4e to e7e466f Compare September 27, 2024 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chainsaw Pull requests or issues owned by Team Chainsaw work-in-progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants